feat: authorization-zone intents for third-party keepers (#370) - #373
feat: authorization-zone intents for third-party keepers (#370)#373AbuJulaybeeb wants to merge 1 commit into
Conversation
josephchimebuka
left a comment
There was a problem hiding this comment.
Review — Request changes
Solid direction on the intent/keeper framework and threat-model doc. There are blocking auth bugs that make the keeper paths fail (or worse, unsafe) outside mock_all_auths(), plus a privilege-model issue on Resolve / CreateNext.
Blocking
-
Keeper execution calls auth-gated entrypoints that require a different signer
execute_keeper_claim→claim_winnings→user.require_auth()execute_keeper_resolve→resolve_round→oracle.require_auth()execute_keeper_create_next→betting::create_round→admin.require_auth()
Only the keeper signs these txs. In production (no
mock_all_auths), these will fail auth. Tests pass only becausesetup_env()mocks all auths, which hides the bug.Fix: add internal helpers that skip the original
require_authafter intent validation, e.g.:_claim_winnings_for_user(env, user)(nouser.require_auth, destination still hard-coded touser)_resolve_round_as_oracle(env, payload)or require the intent authorizer to be the oracle and keep oracle auth intentionally_create_next_from_template_internal(env)(noadmin.require_auth)
Prefer CEI: mark intent
Consumed(or write a “in-flight” lock) before external effects if any path can partially succeed; today consume is after the call (OK under Soroban atomicity, but document it). -
Who may authorize high-privilege scopes?
Any user can callauthorize_keeper_intent(..., Resolve|CreateNext, ...).Resolveis normally oracle-onlyCreateNext/create_roundis normally admin-only
If you fix (1) by bypassing
oracle/adminauth after a user-issued intent, that is a privilege escalation (user-nominated keeper becomes de-facto oracle/admin).Fix: restrict authorization:
Claim→ user may authorizeResolve→ only current oracle (or admin) may authorizeCreateNext→ only admin may authorize
Add tests that a random user cannot authorize Resolve/CreateNext.
-
Tombstone never checked
_consume_intentwritesConsumedIntentNonce, but_check_intent_activeonly readsstatus. Either check the tombstone on execute, or drop the unused key to avoid false confidence in the threat model (§3.1).
High
-
PR description vs code constants
Description saysMAX_INTENT_EXPIRY_LEDGERS = 172,800; code uses1_036_800. Align docs/PR/INTENT_THREAT_MODEL.md. -
Bindings error codes
IntentAlreadyConsumedErroruses code79inhelpers.ts, while other work on main uses79forAccessDenied. Reconcile withContractErrorso clients map correctly. -
Missing tests for resolve happy path / auth failure
No test thatexecute_keeper_resolveworks end-to-end, and no test with selective auth (mock only keeper, not user/oracle/admin) proving claim/resolve/create_next work as designed.
Medium / nits
- Scope-isolation test expects
IntentNotFound(key includes scope) — fine, but also assertIntentScopeMismatchif you ever look up by nonce alone. create_nextpath loads template then callscreate_roundinstead ofcreate_next_from_template— inconsistent with the public API and PR description.- Large mechanical churn in
types.rs/ unrelated modules — please keep this PR scoped to intents + minimal glue, or call out unavoidable storage-split changes clearly.
Suggested acceptance checklist before re-review
- Internal auth-bypass helpers for keeper paths (or intentional co-sign model documented + tested)
- Restrict Resolve/CreateNext intent authorization to oracle/admin
- Auth-selective tests (no
mock_all_authsfor the execute step) - Tombstone checked or removed; constants/docs aligned
- Bindings error codes match
ContractError
Happy to re-review quickly once those are addressed.
Summary
execute_keeper_resolve: permissioned oracle payload resolution under an activeKeeperScope::Resolveintent.execute_keeper_claim: user winning claims executed by an authorized keeper under aKeeperScope::Claimintent, guaranteeing that all claimed winnings are credited directly to the user's custody (accumulate_pending).execute_keeper_create_next: automated template-based round rollover executed under aKeeperScope::CreateNextintent.KeeperScope) preventing privilege escalation across action domains.expires_at_ledger) with validation (MIN_INTENT_EXPIRY_LEDGERS = 6,MAX_INTENT_EXPIRY_LEDGERS = 172,800).revoke_keeper_intent).register_keeper,deregister_keeper,set_keeper_registration_required).docs/INTENT_THREAT_MODEL.md.bindings/src/helpers.ts.Why
Third-party automation services (keepers, bots, relayer networks) need the ability to perform operational tasks (settling expired rounds, claiming user rewards, rolling over rounds) without:
This capability-security authorization framework enforces strict scoped permissions, replay resistance, expiry windows, and revocation safety.
Implementation
Core Intent Module (
contracts/src/intents.rs):authorize_keeper_intent(env, user, keeper, scope, duration_ledgers) -> u64: Authorizes a keeper with a monotonic per-scope nonce and expiry ledger.revoke_keeper_intent(env, user, scope, nonce) -> Result<(), ContractError>: Explicit user revocation.get_keeper_intent(env, user, scope, nonce) -> Option<KeeperIntent>: Observability query.execute_keeper_resolve(env, keeper, user, nonce, payload) -> Result<(), ContractError>: Gated resolution.execute_keeper_claim(env, keeper, user, nonce) -> Result<i128, ContractError>: Gated claim preserving user custody.execute_keeper_create_next(env, keeper, user, nonce) -> Result<u64, ContractError>: Gated round rollover.register_keeper,deregister_keeper,set_keeper_registration_required,is_keeper_registered,is_keeper_registration_required: Optional keeper allowlist gating.Types & Storage (
contracts/src/types.rs,contracts/src/errors.rs):KeeperScope(Resolve,Claim,CreateNext),KeeperIntentStatus(Active,Consumed,Revoked),KeeperIntent,IntentKey.ContractErrorcodes 80–87 for intent error handling.ContractErrorto handle the SDK error conversion and variant limits safely.Contract Interface (
contracts/src/contract.rs,contracts/src/lib.rs):VirtualTokenContract.Test Suite (
contracts/src/tests/intents.rs):Testing
Executed automated test suite:
cargo check --lib(clean build)cargo test --lib -- tests::intents